Skip to content

Python 3 compatibility #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Python 3 compatibility #18

wants to merge 7 commits into from

Conversation

smcoll
Copy link

@smcoll smcoll commented May 31, 2017 •
edited
Loading

Tests pass for Python 3.2-3.5, so let's get it marked as 3-compatible.

For the record httpsig doesn't indicate that it supports 3.5, but the tests here pass.

Note that i am interpreting string literals as unicode for Python 2, which is technically a change in behavior. i don't foresee a problem with that, and this makes behavior consistent between 2 and 3.

Also, i made the regex match non-greedy because depending on the order of the header values, it was at times matching too much. For example:

>>> sig_string = 'Signature keyId="su-key",algorithm="hmac-sha256",signature="+dV3yojX7N5I5J+rx0N+7kL5zES2L9Goo4ApJIn33IM=",headers="(request-target) host accept date"'
>>> match = re.match(r'.*signature="(.+)",?.*', sig_string)
>>> match.groups()
('+dV3yojX7N5I5J+rx0N+7kL5zES2L9Goo4ApJIn33IM=",headers="(request-target) host accept date',)
>>> # now with updated regex:
match = re.match(r'.*signature="(.*?)",?.*', sig_string)
>>> match.groups()
('+dV3yojX7N5I5J+rx0N+7kL5zES2L9Goo4ApJIn33IM=',)

@smcoll
Copy link
Author

smcoll commented May 31, 2017

i believe the failing test is due to incompatibility with Django 1.11, right?

@smcoll
Copy link
Author

smcoll commented May 31, 2017

i don't believe ROOT_URLCONF is required in any supported version of Django, and i removed it to get tests passing for 1.11. Added 3.2-3.5 to the Travis config.

@smcoll
Copy link
Author

smcoll commented May 31, 2017

i didn't want to figure out how to match the latest relevant Django version to each Python version, so i dropped 3.2 and 3.3 to get CI passing.

For reference, here are the currently supported Django versions and the Python versions they support:

Django 1.11: 2.7, 3.4, 3.5, 3.6
Django 1.10: 2.7, 3.4, 3.5
Django 1.8: 2.7, 3.2, 3.3, 3.4, 3.5

hugovk
hugovk approved these changes Oct 20, 2017
View reviewed changes
setup.py Outdated
@@ -25,6 +25,8 @@
'Framework :: Django',
'Operating System :: OS Independent',
'Programming Language :: Python :: 2.7',
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add these?

'Programming Language :: Python :: 2',
'Programming Language :: Python :: 3',
'Programming Language :: Python :: Implementation :: CPython',

@hugovk
Copy link

hugovk commented Oct 20, 2017

Sounds fine dropping Python 3.2 and 3.3, they're both EOL.

Here's the pip installs for djangorestframework-httpsignature from PyPI for the last month:

$ pypinfo --percent --pip djangorestframework-httpsignature pyversion
python_version percent download_count
-------------- ------- --------------
3.6              39.6%            243
2.7              33.9%            208
3.5              26.4%            162
3.4               0.2%              1

@smcoll
Copy link
Author

smcoll commented Oct 20, 2017

Ah, we should add 3.6 now...

@smcoll
Copy link
Author

smcoll commented Oct 20, 2017

@hugovk look ok now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugovk hugovk hugovk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@smcoll @hugovk